Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Dockerfile and workflow #8

Merged
merged 6 commits into from
Nov 21, 2024

Conversation

andy5995
Copy link
Contributor

How do you install clang-19 on Trixie?

@KaruroChori
Copy link
Owner

KaruroChori commented Nov 19, 2024

I think I followed the instructions here https://apt.llvm.org/
I most surely used the automatic installer

wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh 19

Depending on the base image used, many packages needed for development are missing. If I recall correcty debian is much more basic compared to the full ubuntu image here on github actions. It might not even have git out of the box

Otherwise, appending

`deb http://apt.llvm.org/unstable/ llvm-toolchain-19 main
# deb-src http://apt.llvm.org/unstable/ llvm-toolchain-19 main

to /etc/apt/sources.list should be enough to make it available

@andy5995
Copy link
Contributor Author

I'm using trixie-slim for the base image.

I found that clang-19 is actually available without adding anything to the source list. :)

wget https://apt.llvm.org/llvm.sh

This doesn't work because it requires add-apt-repository and sudo apt install software-properties-common be installed. apt using the trixie-slim image only told me those package weren't available.

@andy5995 andy5995 force-pushed the iss-7-docker branch 3 times, most recently from e2f0775 to 1a0d77e Compare November 20, 2024 06:22
docs/for-developers.md Outdated Show resolved Hide resolved
@andy5995
Copy link
Contributor Author

The main problem with this as it is: builder may not have privileges to write the build directory or create the node_modules directory. I deal with this issue in this other project I work on by using an entrypoint.sh file. It works out not too bad but maybe me, or you and I, can think of a better solution. But no need to prioritize this PR.

@KaruroChori
Copy link
Owner

KaruroChori commented Nov 20, 2024

This doesn't work because it requires add-apt-repository and sudo apt install software-properties-common be installed.

apt using the trixie-slim image only told me those package weren't available.
I see, I have a mixture of ubuntu and debian systems so at times it gets to remember what worked for each :D.

The main problem with this as it is: builder may not have privileges to write the build directory or create the node_modules directory.

Not sure if I understood correctly, but the flatpak builder expects bun install and bun run codegen to run externally, before starting its own building process. Unless you are considering a different builder in this context.

@andy5995
Copy link
Contributor Author

apt using the trixie-slim image only told me those package weren't available. I see, I have a mixture of ubuntu and debian systems so at times it gets to remember what worked for each :D.

I can relate to that! :)

The main problem with this as it is: builder may not have privileges to write the build directory or create the node_modules directory.

Not sure if I understood correctly, but the flatpak builder expects bun install and bun run codegen to run externally, before starting its own building process. Unless you are considering a different builder in this context.

I'm kind of shot for the night but I'll try to elaborate. The solution might be very simple after I get some rest. Or you'll have it all figured out, but

When the image is run with the -v option (as in the examples I've documented), it's mounting the source directory. All commands are executed as 'builder', the user created in the Dockerfile. 'builder' may not have permissions to create directories because he/she/they/zirs has a different UID than the source directory.

In the entrypoint.sh file I linked to above, I solve this by having root (in the container) change the UID of builder to the UID of whoever started the container.

@KaruroChori
Copy link
Owner

No problem :D. I will need some time to read that and the example your posted properly, thank you for the additional context.

@andy5995 andy5995 force-pushed the iss-7-docker branch 2 times, most recently from 86ea7c7 to 4f4ba59 Compare November 20, 2024 22:27
@andy5995
Copy link
Contributor Author

While building, not finding entrypoint.sh. I have no idea why. It works locally.
https://github.com/KaruroChori/vs-fltk/actions/runs/11942830766/job/33290659332#step:3:60

I'll get back to it later.

@andy5995
Copy link
Contributor Author

While building, not finding entrypoint.sh. I have no idea why.

I had duplicate build steps that conflicted. Fixed.

Remaining issues:

  • The entrypoint.sh doesn't work as expected when no arguments are given (just give a shell, please)
  • The docs need to be revised now that entrypoint is being used and host{uid,gid} need to be provided
  • Add a job to the regular build pipeline that tests using the image
  • Probably a script will be preferred to start the container
  • Decide on if we want the docker-compose.yml file at all
    • If so, the HOSTUID and HOSTGID could be put in an .env file so the dev won't have to add them to the command line
    • Or a script that dynamically passes UID and GID and runs docker-compose

Probably some other minor issues :)

@andy5995 andy5995 marked this pull request as ready for review November 21, 2024 08:02
@andy5995
Copy link
Contributor Author

@KaruroChori I think what I have here so far is gonna be pretty close to the end result. I've converted this from draft to review. I'll do a bit more testing and doc proof-reading much later on today.

- '**docker.yml'
# workflow_dispatch:
# schedule:
# - cron: '30 11 20 */3 *'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How often would you like this automatically built?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any suggestion for that?
I think once each fortnight is probably enough, possibly even too much.
I mean, if there are good reasons to have a new image asap we can just trigger it manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I thought a fortnight as well, although since I'm an American I actually thought two weeks. ;) But... monthly should be alright, because as you said, we can trigger it manually, or change the cron setting

@KaruroChori
Copy link
Owner

Great! I will also be reviewing & testing it later today :D.

docs/for-developers.md Outdated Show resolved Hide resolved
@KaruroChori KaruroChori merged commit 74c633c into KaruroChori:master Nov 21, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants